refactor: make block processing async inside PendingBlocks #709
Conversation
| @@ -1,40 +0,0 @@ | |||
| defmodule Unit.PendingBlocks do | |||
| # If parent is not in fork choice, download parent | ||
| not ForkChoice.has_block?(parent_root) -> | ||
| state |> Map.update!(:blocks_to_download, &MapSet.put(&1, parent_root)) | ||
| !Blocks.get_block(parent_root) -> |
There was a problem hiding this comment.
This introduces a bug: if the parent is in the DB this would be false, and we'd try to process the block even if its parent wasn't processed, causing it to fail and marking it as an invalid block
There was a problem hiding this comment.
But this assumes that a block is stored in the DB only if it is processed (and valid). We should not store invalid (or potentially invalid) blocks.
Do we store blocks before processing them today?
There was a problem hiding this comment.
No. But this can happen if the node restarts after processing some blocks
There was a problem hiding this comment.
I think that this is probably out of scope from this PR
There was a problem hiding this comment.
For sure. We could patch it up by just nuking the database in the makefile to avoid this error when debugging, but we need to find a better way to handle this.
There was a problem hiding this comment.
Would you like me to add this to the PR?
There was a problem hiding this comment.
I think this is a more complex discussion outside of the scope of the PR, which is supporting recoverability, which we don't yet. To do so, we should save the state of a block in the DB.
There was a problem hiding this comment.
Just to clarify, I meant we need to find a better way to handle this in a following PR. For now, we can just go for the nuclear option.
…us into refactor-pending-blocks
…us into refactor-pending-blocks
Motivation
Before this change, PendingBlocks was waiting for ForkChoice to process the block before updating it's status. This sometimes results in a timeout if there are many blocks that need to be processed, causing the PendingBlocks genserver to crash.
Description